What this PR does

It removes duplicated code inside the core/membership package. It builds on #1826, which split the one big service file into smaller files. With the code side by side, the copies were easy to see.

No results change. Every public function keeps its name and signature. The package shrinks by 165 lines. One function makes one less database query (change #9, from review).

The duplications, one by one

1. Two functions built the same policy filter.
resourcePolicyFilter (list.go) and policyFilterForResource (service.go) did the same job: take a resource ID and type, and set the matching field on policy.Filter (OrgID, ProjectID, or GroupID). Now only resourcePolicyFilter exists. removeAllPolicies uses it, and then calls the existing removePoliciesByFilter instead of keeping its own copy of the list-then-delete loop.

One side effect to know about: the filter built by removeAllPolicies now also sets ResourceType. The query result is the same. The repository already pins the resource type whenever OrgID/ProjectID/GroupID is set (see applyListFilter in internal/store/postgres/policy_repository.go), so the new condition is redundant, not different.

2. Three role validators were the same function.
validateOrgRole, validateProjectRole, and validateGroupRole all did the same three steps: fetch the role, check it is scoped to the right namespace, then accept it if it is a platform-wide role or a custom role of this org. Only the namespace and the returned error differed. The shared steps now live in validateRoleForScope. The three old functions stay as one-line wrappers, so call sites read the same as before.

3. Two "don't remove the last owner" checks were the same function.
validateMinOwnerConstraint (org) and validateMinGroupOwnerConstraint (group) ran the same algorithm: fetch the owner role, skip the check if the member is being promoted to owner or isn't an owner, otherwise count the owner policies and fail if this member is the only one. Both now call the shared validateMinRoleConstraint. Each wrapper passes its own owner role name, filter, and error.

4. Five audit helpers repeated the same ~30 lines.
The org and group helpers for member added, role changed, and member removed each built the same audit record (target, metadata with role_id and email) and then wrote the same legacy audit log. That shared body is now one function, auditMemberChange. The five helpers became short wrappers that only supply the event names and attributes. Adding the next membership event is a five-line wrapper.

5. A repeated condition got a name.
len(existing) == 1 && existing[0].RoleID == roleID appeared at four call sites. It now reads hasExactlyRole(existing, roleID), which says what the check means: the member already has exactly this role, so there is nothing to change.

6. String-glued map keys became a struct.
Two places built map keys by joining strings: principalType + "\x00" + principalID. They now use a small struct, principalKey{Type, ID}. Go structs work directly as map keys, and there is no separator character to reason about.

7. The longest function got split.
ListPrincipalsByResource was ~85 lines doing two jobs: find the members, then fetch every role each member holds. The second job moved into its own function, enrichMemberRoles.

8. Hand-written loops replaced by an existing helper.
listOrgsForPrincipal and listGroupsForPrincipal each looped over policies to collect resource IDs and dedupe them. The helper policyResourceIDs already did exactly that, so they call it now.

9. One less query in ListPrincipalsByResource (from review).
CodeRabbit spotted a pre-existing inefficiency that the split made visible: when the caller passes no role filter, the role-enrichment query was identical to the member query, so the function hit the database twice for the same rows. Now the first result is reused; only a role-filtered listing issues the second, unfiltered query. Same results, one query instead of two on the common path.

What I looked at but did not merge

validateOrgMembership and validatePrincipal overlap, but they return different errors (ErrNotOrgMember vs ErrPrincipalNotInOrg), and only one of them checks whether a service user is disabled. Merging them would change behavior, and this PR promises not to. Left as is.

Tests

Two kinds of test changes, both matching the changes above:

• The four mock expectations in TestService_RemoveProjectMember now expect ResourceType: schema.ProjectNamespace in the filter (change feat: add username and groupname columns #1).
• The four no-role-filter cases in TestService_ListPrincipalsByResource now pin the policy List call with .Once() instead of .Times(2) (change chore(deps): bump normalize-url from 4.5.0 to 4.5.1 #9), proving the duplicate query is gone.

Everything else passes untouched. go test ./core/... ./internal/api/... is green.

🤖 Generated with Claude Code